Skip to content

gobject-introspection: support cross-compilation #176464

Merged
Artturin merged 5 commits intoNixOS:stagingfrom
Artturin:gobjecfix
Jun 12, 2022
Merged

gobject-introspection: support cross-compilation #176464
Artturin merged 5 commits intoNixOS:stagingfrom
Artturin:gobjecfix

Conversation

@Artturin
Copy link
Member

@Artturin Artturin commented Jun 5, 2022 •
edited
Loading

with this json-glib cross-compiles and the gir files exist, have not tested running any binaries on aarch64

nix build ".#pkgsCross.aarch64-multiplatform.json-glib"

#88222
#72868

https://github.com/void-linux/void-packages/blob/master/srcpkgs/gobject-introspection/template and the buildroot mk file were inspirations

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Artturin Artturin added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jun 5, 2022
@Artturin Artturin requested a review from Mindavi June 5, 2022 20:04
@Artturin Artturin marked this pull request as draft June 5, 2022 20:04
@Artturin Artturin requested a review from Ericson2314 June 5, 2022 20:04
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jun 5, 2022
@Mindavi
Copy link
Contributor

Mindavi commented Jun 5, 2022

Hmm, will we need to build such wrapper for each package using gobject-introspection?

SuperSandro2000
SuperSandro2000 reviewed Jun 5, 2022
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some formatting improvements. Thanks for trying to bring this forward. I really really appreciate it!

@ofborg ofborg bot requested review from 7c6f434c and edolstra June 6, 2022 01:52
Artturin
Artturin commented Jun 6, 2022
View reviewed changes
@Mindavi Mindavi mentioned this pull request Jun 6, 2022
Merged
9 tasks
@Mindavi
Copy link
Contributor

Mindavi commented Jun 7, 2022

I do like this better than having it in json-glib at least :). Still need to try it out

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 8.has: clean-up This PR removes packages or removes other cruft labels Jun 7, 2022
@Artturin Artturin changed the title WIP: gobject-introspection fix cross gobject-introspection: support cross-compilation Jun 7, 2022
Mindavi
Mindavi reviewed Jun 7, 2022
View reviewed changes
SuperSandro2000
SuperSandro2000 reviewed Jun 7, 2022
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are in a good state here now!

@Mindavi what do you think? Can we merge this soon?

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Jun 8, 2022
@Artturin
Copy link
Member Author

Artturin commented Jun 9, 2022

I think we are in a good state here now!
@Mindavi what do you think? Can we merge this soon?

I think we can merge this soon-ish. I want to do some testing, but I tried out an older version (from yesterday) and that was already looking promising. Let's also address the review comments 😄.

Just to get some idea of the status:

1. For any package _not_ using meson, we don't need to add the hook, right? It should work out of the box?

2. For any package using meson we need the hook because meson does it's own handing of gobject-introspection, correct?

I'm not really sure if we really want to make a separate hook or if we always want to do this when meson and gobject-introspection are both used. I think it would be more convenient and cause less friction if we could automatically add the cross-file when needed instead of having to explicitly define it.

In general, most of this is looking good!

The hook is not always needed even with meson, small example
json-glib doesn't need it while gtk3 does

@Mindavi
Copy link
Contributor

Mindavi commented Jun 9, 2022

Cool, yeah, if it's not always needed I think it's preferred to not have to have an emulator unless it's really needed.

@Artturin Artturin force-pushed the gobjecfix branch 2 times, most recently from 52d21bf to 047597d Compare June 10, 2022 16:35
Mindavi
Mindavi reviewed Jun 10, 2022
View reviewed changes
@Artturin Artturin force-pushed the gobjecfix branch 2 times, most recently from ebf1f89 to 8e18a52 Compare June 10, 2022 17:37
@Artturin Artturin requested review from Ericson2314, Mindavi and SuperSandro2000 and removed request for Mindavi and SuperSandro2000 June 10, 2022 20:10
SuperSandro2000
SuperSandro2000 reviewed Jun 11, 2022
View reviewed changes
SuperSandro2000
SuperSandro2000 reviewed Jun 11, 2022
View reviewed changes
SuperSandro2000
SuperSandro2000 reviewed Jun 11, 2022
View reviewed changes
Artturin added 5 commits June 11, 2022 04:51
@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 12, 2022
@Artturin Artturin mentioned this pull request Jun 12, 2022
Closed
13 tasks
Mindavi
Mindavi approved these changes Jun 12, 2022
View reviewed changes
Copy link
Contributor

@Mindavi Mindavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff now LGTM

@Artturin Artturin merged commit ce62ff7 into NixOS:staging Jun 12, 2022
@Artturin Artturin deleted the gobjecfix branch June 12, 2022 14:32
@Artturin
Copy link
Member Author

Artturin commented Jun 12, 2022

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@Mindavi Mindavi Mindavi approved these changes

@bobby285271 bobby285271 Awaiting requested review from bobby285271

@amaxine amaxine Awaiting requested review from amaxine

@lovek323 lovek323 Awaiting requested review from lovek323

@dasj19 dasj19 Awaiting requested review from dasj19

@hedning hedning Awaiting requested review from hedning

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

@edolstra edolstra Awaiting requested review from edolstra

@jtojnar jtojnar Awaiting requested review from jtojnar

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

Assignees

No one assigned

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Artturin @Mindavi @jtojnar @Ericson2314 @SuperSandro2000 @bobby285271